Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

stylus_benchmark #2827

Open
wants to merge 57 commits into
base: master
Choose a base branch
from
Open

stylus_benchmark #2827

wants to merge 57 commits into from

Conversation

diegoximenes
Copy link
Contributor

@diegoximenes diegoximenes commented Dec 11, 2024 •
edited
Loading

Resolves NIT-2757

This PR adds the stylus_benchmark binary.
It will mainly be used to fine tune the ink prices that are charged today.

It deterministically creates .wat programs with lots of instructions to be benchmarked.

If this PR is approved then more .wat programs generations, benchmarking other instructions, will be added in a future PR.

This PR introduces two new WASM instruction to jit, start_benchmark and end_benchmark.
Code blocks between start_benchmark and end_benchmark instructions will be benchmarked.

stylus_benchmark uses jit as a library.

@cla-bot cla-bot bot added the s Automatically added by the CLA bot if the creator of a PR is registered as having signed the CLA. label Dec 11, 2024
@diegoximenes diegoximenes requested a review from tsahee December 11, 2024 19:27
@diegoximenes
wip
fdb42c8
@diegoximenes
Revert "wip"
6705932 
This reverts commit 4fd9d10.
@diegoximenes
jit as a library and binary
d94665a
@diegoximenes
Beginning of stylus_benchmark
cd1642d
@diegoximenes
stylus_benchmark: create_stylus_config and create_evm_data2
25a770b
@diegoximenes
Refactors jit to have *_with_wasm functions
472a662
@diegoximenes
Basic CallProgramLoop in stylus_benchmark
f327531
@diegoximenes
Adds debug.toggle_measurement instruction
2aa59c9
@diegoximenes
toggle_measurement ink tracking
61620f9
@diegoximenes
Run benchmark multiple times for the same wat file
5e17f53
@diegoximenes
toggle_measurement to toggle_benchmark
89eeee8
@diegoximenes
Adds whole Benchmark object to MessageFromCothread
7d9819c
@diegoximenes
Stores benchmark as an Option<Benchmark> in WasmEnv
55463ed
@diegoximenes
reuse compiled module
e9a74d9
@diegoximenes
Uses ? instead of unwrap when appropriate
1198dfa
@diegoximenes
Small refactor on stylus_benchmark
d53bcc2
@diegoximenes
stylus_benchmark: from to_result to check_result
d2668f6
@diegoximenes
Removes send_response_with_wasm_env
c4fb5df
@diegoximenes
Removes cycle from Benchmark
f36013f
@diegoximenes
Renames add_one.wat to add_i32.wat. Uses loop in add_i32.wat
24dac3e
@diegoximenes
stylus_benchmark: print ink usage
3df9b24
@diegoximenes
Improves prints in stylus_benchmark
87f7386
@diegoximenes
fix cargo fmt
654d18a
@diegoximenes
fix cargo fmt
15ca778
@diegoximenes
Adds rust fmt to stylus_benchmark in ci
a813433
@diegoximenes
Improve panics msgs in stylus_benchmark
1f1e5e3
@diegoximenes
generate-wats subcommand
175e39b
@diegoximenes
Split benchmark and generate_wats in different files
922fce8
@tsahee
Copy link
Contributor

tsahee commented Dec 17, 2024

general approach seems great.
would like to get more reviews but I think it's already interesting to add a few benchmarks and start looking at results.

gligneul
gligneul reviewed Dec 18, 2024
View reviewed changes
arbitrator/tools/stylus_benchmark/src/generate_wats.rs Outdated Show resolved Hide resolved
arbitrator/arbutil/src/benchmark.rs Outdated Show resolved Hide resolved
arbitrator/arbutil/src/benchmark.rs Show resolved Hide resolved
arbitrator/tools/stylus_benchmark/src/benchmark.rs Outdated Show resolved Hide resolved
arbitrator/tools/stylus_benchmark/src/benchmark.rs Outdated

let exec = &mut WasmEnv::default();

let module = exec_program(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this code benchmarking only the JIT execution? I'm worried about the differences between the JIT and the interpreter, so it would be nice to benchmark non-JIT execution as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a good point.
By interpreter you mean execution through the prover's binary?

@tsahee, any takes on that?

@diegoximenes
Copy link
Contributor Author

diegoximenes commented Dec 23, 2024 •
edited
Loading

general approach seems great. would like to get more reviews but I think it's already interesting to add a few benchmarks and start looking at results.

I intend to add more scenarios to be benchmarked in a future PR, together with a more detailed look at results.

But here goes the output of the benchmarks implemented in this PR, that was run in a Apple M3 Pro:

No scenario specified, benchmarking all scenarios

Benchmarking add_i32
Run 0, duration: 120.735917ms, ink_spent: Ink(29298411152)
Run 1, duration: 108.255875ms, ink_spent: Ink(29298411152)
Run 2, duration: 110.482833ms, ink_spent: Ink(29298411152)
Run 3, duration: 95.918417ms, ink_spent: Ink(29298411152)
Run 4, duration: 95.604125ms, ink_spent: Ink(29298411152)
Run 5, duration: 95.353792ms, ink_spent: Ink(29298411152)
Run 6, duration: 95.531375ms, ink_spent: Ink(29298411152)
After discarding top and bottom runs:
avg_duration: 99.926139ms, avg_ink_spent_per_micro_second: 293201
Benchmarking xor_i32
Run 0, duration: 96.054583ms, ink_spent: Ink(29298411152)
Run 1, duration: 96.208625ms, ink_spent: Ink(29298411152)
Run 2, duration: 97.552791ms, ink_spent: Ink(29298411152)
Run 3, duration: 96.658ms, ink_spent: Ink(29298411152)
Run 4, duration: 96.164417ms, ink_spent: Ink(29298411152)
Run 5, duration: 96.1905ms, ink_spent: Ink(29298411152)
Run 6, duration: 96.149083ms, ink_spent: Ink(29298411152)
After discarding top and bottom runs:
avg_duration: 96.187847ms, avg_ink_spent_per_micro_second: 304598

gligneul
gligneul approved these changes Dec 23, 2024
View reviewed changes
Copy link
Contributor

@gligneul gligneul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I think measuring the interpreter (prover) performance is important as well, but you can leave this for a future PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gligneul gligneul gligneul approved these changes

@magicxyyz magicxyyz Awaiting requested review from magicxyyz

@ganeshvanahalli ganeshvanahalli Awaiting requested review from ganeshvanahalli

@tsahee tsahee Awaiting requested review from tsahee

Assignees
No one assigned
Labels
s Automatically added by the CLA bot if the creator of a PR is registered as having signed the CLA.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@diegoximenes @tsahee @gligneul